Added support for included groups This change adds a new item to the group configuration: a list of groups whose members should be included in this one. This makes it possible to set up a hierarchy of included groups, which can make it easier to maintain complex access control lists. To accomplish this, two new database tables were added, called AccountGroupIncludes and AccountGroupIncludesAudit. The relevant support code was added around them, largely based on the existing code for handling indivdual account membership. In addition, caches for group information were added, paralleling the caches that already exist for accounts. Change-Id: Ib6990c17739f28f38bc13961143db7ce79251567
diff --git a/Documentation/access-control.txt b/Documentation/access-control.txt index 2862287..20fe52d 100644 --- a/Documentation/access-control.txt +++ b/Documentation/access-control.txt
@@ -3,8 +3,8 @@ Access controls in Gerrit are group based. Every user account is a member of one or more groups, and access and privileges are granted -to those groups. Groups cannot be nested, and access rights cannot -be granted to individual users. +to those groups. Access rights cannot be granted to individual +users. System Groups @@ -88,9 +88,6 @@ added individually by a group owner. Any user account listed as a group member is given any access rights granted to the group. -To keep the schema simple to manage, groups cannot be nested. -Only individual user accounts can be added as a member. - Every group has one other group designated as its owner. Users who are members of the owner group can:
diff --git a/Documentation/cmd-create-group.txt b/Documentation/cmd-create-group.txt index d5bc757..1e46e14 100644 --- a/Documentation/cmd-create-group.txt +++ b/Documentation/cmd-create-group.txt
@@ -12,6 +12,7 @@ [\--owner <GROUP>] \ [\--description <DESC>] \ [\--member <USERNAME>] \ +[\--group <GROUP>] \ <GROUP> DESCRIPTION @@ -50,6 +51,10 @@ User name to become initial member of the group. Multiple \--member options may be specified to add more initial members. +\--group:: + Group name to include in the group. Multiple \--group options may + be specified to include more initial groups. + EXAMPLES -------- Create a new account group called `gerritdev` with two initial members
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 7a116c4..bebce29 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt
@@ -366,6 +366,11 @@ table is cached under the `"accounts"` cache, above. External group membership obtained from LDAP is cached under `"ldap_groups"`. +cache `"groups_byinclude"`:: ++ +Caches group inclusions in other groups. If direct updates are made +to the `account_group_includes` table, this cache should be flushed. + cache `"ldap_groups"`:: + Caches the LDAP groups that a user belongs to, if LDAP has been
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupAdminService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupAdminService.java index 445bdb4..ce508cc 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupAdminService.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupAdminService.java
@@ -16,6 +16,7 @@ import com.google.gerrit.common.auth.SignInRequired; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwtjsonrpc.client.RemoteJsonService; @@ -70,6 +71,14 @@ AsyncCallback<GroupDetail> callback); @SignInRequired + void addGroupInclude(AccountGroup.Id groupId, String groupName, + AsyncCallback<GroupDetail> callback); + + @SignInRequired void deleteGroupMembers(AccountGroup.Id groupId, Set<AccountGroupMember.Key> keys, AsyncCallback<VoidResult> callback); + + @SignInRequired + void deleteGroupIncludes(AccountGroup.Id groupId, + Set<AccountGroupInclude.Key> keys, AsyncCallback<VoidResult> callback); }
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java index 5ddca26..69e535c 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupDetail.java
@@ -15,14 +15,17 @@ package com.google.gerrit.common.data; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; import com.google.gerrit.reviewdb.AccountGroupMember; import java.util.List; public class GroupDetail { public AccountInfoCache accounts; + public GroupInfoCache groups; public AccountGroup group; public List<AccountGroupMember> members; + public List<AccountGroupInclude> includes; public AccountGroup ownerGroup; public boolean canModify; @@ -33,6 +36,10 @@ accounts = c; } + public void setGroups(GroupInfoCache c) { + groups = c; + } + public void setGroup(AccountGroup g) { group = g; } @@ -41,6 +48,10 @@ members = m; } + public void setIncludes(List<AccountGroupInclude> i) { + includes = i; + } + public void setOwnerGroup(AccountGroup g) { ownerGroup = g; } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java new file mode 100644 index 0000000..55577f2 --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfo.java
@@ -0,0 +1,64 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.common.data; + +import com.google.gerrit.reviewdb.AccountGroup; + +/** Summary information about an {@link AccountGroup}, for simple tabular displays. */ +public class GroupInfo { + protected AccountGroup.Id id; + protected String name; + protected String description; + + protected GroupInfo() { + } + + /** + * Create an anonymous group info, when only the id is known. + * <p> + * This constructor should only be a last-ditch effort, when the usual group + * lookup has failed and a stale group id has been discovered in the data + * store. + */ + public GroupInfo(final AccountGroup.Id id) { + this.id = id; + } + + /** + * Create a group description from a real data store record. + * + * @param a the data store record holding the specific group details. + */ + public GroupInfo(final AccountGroup a) { + id = a.getId(); + name = a.getName(); + description = a.getDescription(); + } + + /** @return the unique local id of the group */ + public AccountGroup.Id getId() { + return id; + } + + /** @return the name of the group; null if not supplied */ + public String getName() { + return name; + } + + /** @return the description of the group; null if not supplied */ + public String getDescription() { + return description; + } +} diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfoCache.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfoCache.java new file mode 100644 index 0000000..80d8756 --- /dev/null +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GroupInfoCache.java
@@ -0,0 +1,79 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.common.data; + +import com.google.gerrit.reviewdb.AccountGroup; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** In-memory table of {@link GroupInfo}, indexed by {@link AccountGroup.Id}. */ +public class GroupInfoCache { + private static final GroupInfoCache EMPTY; + static { + EMPTY = new GroupInfoCache(); + EMPTY.groups = Collections.emptyMap(); + } + + /** Obtain an empty cache singleton. */ + public static GroupInfoCache empty() { + return EMPTY; + } + + protected Map<AccountGroup.Id, GroupInfo> groups; + + protected GroupInfoCache() { + } + + public GroupInfoCache(final Iterable<GroupInfo> list) { + groups = new HashMap<AccountGroup.Id, GroupInfo>(); + for (final GroupInfo gi : list) { + groups.put(gi.getId(), gi); + } + } + + /** + * Lookup the group summary + * <p> + * The return value can take on one of three forms: + * <ul> + * <li><code>null</code>, if <code>id == null</code>.</li> + * <li>a valid info block, if <code>id</code> was loaded.</li> + * <li>an anonymous info block, if <code>id</code> was not loaded.</li> + * </ul> + * + * @param id the id desired. + * @return info block for the group. + */ + public GroupInfo get(final AccountGroup.Id id) { + if (id == null) { + return null; + } + + GroupInfo r = groups.get(id); + if (r == null) { + r = new GroupInfo(id); + groups.put(id, r); + } + return r; + } + + /** Merge the information from another cache into this one. */ + public void merge(final GroupInfoCache other) { + assert this != EMPTY; + groups.putAll(other.groups); + } +} \ No newline at end of file diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/errors/NoSuchGroupException.java b/gerrit-common/src/main/java/com/google/gerrit/common/errors/NoSuchGroupException.java index f7115b9..4e117b1 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/errors/NoSuchGroupException.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/errors/NoSuchGroupException.java
@@ -37,4 +37,12 @@ public NoSuchGroupException(final AccountGroup.NameKey k, final Throwable why) { super(MESSAGE + k.toString(), why); } + + public NoSuchGroupException(String who) { + this(who, null); + } + + public NoSuchGroupException(String who, final Throwable why) { + super(MESSAGE + who, why); + } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java index 981fee4..56abefa 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
@@ -14,6 +14,7 @@ package com.google.gerrit.client.admin; +import com.google.gerrit.client.Dispatcher; import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.rpc.GerritCallback; import com.google.gerrit.client.rpc.ScreenLoadCallback; @@ -21,15 +22,20 @@ import com.google.gerrit.client.ui.AccountGroupSuggestOracle; import com.google.gerrit.client.ui.AccountScreen; import com.google.gerrit.client.ui.AddMemberBox; +import com.google.gerrit.client.ui.AddIncludedGroupBox; import com.google.gerrit.client.ui.FancyFlexTable; +import com.google.gerrit.client.ui.Hyperlink; import com.google.gerrit.client.ui.OnEditEnabler; import com.google.gerrit.client.ui.RPCSuggestOracle; import com.google.gerrit.client.ui.SmallHeading; import com.google.gerrit.common.data.AccountInfoCache; import com.google.gerrit.common.data.GroupDetail; import com.google.gerrit.common.data.GroupOptions; +import com.google.gerrit.common.data.GroupInfo; +import com.google.gerrit.common.data.GroupInfoCache; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gwt.event.dom.client.ChangeEvent; import com.google.gwt.event.dom.client.ChangeHandler; @@ -59,7 +65,9 @@ public class AccountGroupScreen extends AccountScreen { private final AccountGroup.Id groupId; private AccountInfoCache accounts = AccountInfoCache.empty(); + private GroupInfoCache groups = GroupInfoCache.empty(); private MemberTable members; + private IncludeTable includes; private NpTextBox groupNameTxt; private Button saveName; @@ -79,6 +87,10 @@ private AddMemberBox addMemberBox; private Button delMember; + private Panel includePanel; + private AddIncludedGroupBox addIncludeBox; + private Button delInclude; + private Panel externalPanel; private Label externalName; private NpTextBox externalNameFilter; @@ -122,6 +134,7 @@ initGroupOptions(); initGroupType(); initMemberList(); + initIncludeList(); initExternal(); } @@ -299,7 +312,7 @@ addMemberBox.addClickHandler(new ClickHandler() { @Override public void onClick(final ClickEvent event) { - doAddNew(); + doAddNewMember(); } }); @@ -321,6 +334,34 @@ add(memberPanel); } + private void initIncludeList() { + addIncludeBox = new AddIncludedGroupBox(); + + addIncludeBox.addClickHandler(new ClickHandler() { + @Override + public void onClick(final ClickEvent event) { + doAddNewInclude(); + } + }); + + includes = new IncludeTable(); + + delInclude = new Button(Util.C.buttonDeleteIncludedGroup()); + delInclude.addClickHandler(new ClickHandler() { + @Override + public void onClick(final ClickEvent event) { + includes.deleteChecked(); + } + }); + + includePanel = new FlowPanel(); + includePanel.add(new SmallHeading(Util.C.headingIncludedGroups())); + includePanel.add(addIncludeBox); + includePanel.add(includes); + includePanel.add(delInclude); + add(includePanel); + } + private void initExternal() { externalName = new Label(); @@ -366,6 +407,7 @@ typeSelect.setVisible(!system); saveType.setVisible(!system); memberPanel.setVisible(newType == AccountGroup.Type.INTERNAL); + includePanel.setVisible(newType == AccountGroup.Type.INTERNAL); externalPanel.setVisible(newType == AccountGroup.Type.LDAP); externalNameFilter.setText(groupNameTxt.getText()); @@ -489,7 +531,9 @@ switch (group.getType()) { case INTERNAL: accounts = result.accounts; + groups = result.groups; members.display(result.members); + includes.display(result.includes); break; case LDAP: @@ -503,7 +547,7 @@ visibleToAllCheckBox.setValue(group.isVisibleToAll()); } - void doAddNew() { + void doAddNewMember() { final String nameEmail = addMemberBox.getText(); if (nameEmail.length() == 0) { return; @@ -529,6 +573,32 @@ }); } + void doAddNewInclude() { + final String groupName = addIncludeBox.getText(); + if (groupName.length() == 0) { + return; + } + + addIncludeBox.setEnabled(false); + Util.GROUP_SVC.addGroupInclude(groupId, groupName, + new GerritCallback<GroupDetail>() { + public void onSuccess(final GroupDetail result) { + addIncludeBox.setEnabled(true); + addIncludeBox.setText(""); + if (result.groups != null && result.includes != null) { + groups.merge(result.groups); + includes.display(result.includes); + } + } + + @Override + public void onFailure(final Throwable caught) { + addIncludeBox.setEnabled(true); + super.onFailure(caught); + } + }); + } + private class MemberTable extends FancyFlexTable<AccountGroupMember> { private boolean enabled = true; @@ -613,4 +683,77 @@ setRowItem(row, k); } } + + private class IncludeTable extends FancyFlexTable<AccountGroupInclude> { + IncludeTable() { + table.setText(0, 2, Util.C.columnGroupName()); + table.setText(0, 3, Util.C.columnGroupDescription()); + + final FlexCellFormatter fmt = table.getFlexCellFormatter(); + fmt.addStyleName(0, 1, Gerrit.RESOURCES.css().iconHeader()); + fmt.addStyleName(0, 2, Gerrit.RESOURCES.css().dataHeader()); + fmt.addStyleName(0, 3, Gerrit.RESOURCES.css().dataHeader()); + } + + void deleteChecked() { + final HashSet<AccountGroupInclude.Key> keys = + new HashSet<AccountGroupInclude.Key>(); + for (int row = 1; row < table.getRowCount(); row++) { + final AccountGroupInclude k = getRowItem(row); + if (k != null && ((CheckBox) table.getWidget(row, 1)).getValue()) { + keys.add(k.getKey()); + } + } + if (!keys.isEmpty()) { + Util.GROUP_SVC.deleteGroupIncludes(groupId, keys, + new GerritCallback<VoidResult>() { + public void onSuccess(final VoidResult result) { + for (int row = 1; row < table.getRowCount();) { + final AccountGroupInclude k = getRowItem(row); + if (k != null && keys.contains(k.getKey())) { + table.removeRow(row); + } else { + row++; + } + } + } + }); + } + } + + void insertMember(final AccountGroupInclude k) { + final int row = table.getRowCount(); + table.insertRow(row); + applyDataRowStyle(row); + populate(row, k); + } + + void display(final List<AccountGroupInclude> result) { + while (1 < table.getRowCount()) + table.removeRow(table.getRowCount() - 1); + + for (final AccountGroupInclude k : result) { + final int row = table.getRowCount(); + table.insertRow(row); + applyDataRowStyle(row); + populate(row, k); + } + } + + void populate(final int row, final AccountGroupInclude k) { + AccountGroup.Id id = k.getIncludeId(); + GroupInfo group = groups.get(id); + table.setWidget(row, 1, new CheckBox()); + table.setWidget(row, 2, new Hyperlink(group.getName(), Dispatcher + .toAccountGroup(id))); + table.setText(row, 3, groups.get(id).getDescription()); + + final FlexCellFormatter fmt = table.getFlexCellFormatter(); + fmt.addStyleName(row, 1, Gerrit.RESOURCES.css().iconCell()); + fmt.addStyleName(row, 2, Gerrit.RESOURCES.css().dataCell()); + fmt.addStyleName(row, 3, Gerrit.RESOURCES.css().dataCell()); + + setRowItem(row, k); + } + } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java index 6738f5b..ca191c6 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
@@ -22,6 +22,8 @@ String defaultBranchName(); String defaultRevisionSpec(); + String buttonDeleteIncludedGroup(); + String buttonAddIncludedGroup(); String buttonDeleteGroupMembers(); String buttonAddGroupMember(); String buttonSaveDescription(); @@ -47,6 +49,7 @@ String headingProjectOptions(); String headingGroupType(); String headingMembers(); + String headingIncludedGroups(); String headingExternalGroup(); String headingCreateGroup(); String headingAccessRights();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties index f7c27b6..94bb87f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
@@ -3,6 +3,8 @@ defaultBranchName = Branch Name defaultRevisionSpec = Revision (Branch or SHA-1) +buttonDeleteIncludedGroup = Delete +buttonAddIncludedGroup = Add buttonDeleteGroupMembers = Delete buttonAddGroupMember = Add buttonRenameGroup = Rename Group @@ -28,6 +30,7 @@ headingProjectOptions = Project Options headingGroupType = Group Type headingMembers = Members +headingIncludedGroups = Included Groups headingExternalGroup = Selected External Group headingCreateGroup = Create New Group headingAccessRights = Access Rights
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AddIncludedGroupBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AddIncludedGroupBox.java new file mode 100644 index 0000000..02e2745 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AddIncludedGroupBox.java
@@ -0,0 +1,104 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.client.ui; + +import com.google.gerrit.client.admin.Util; +import com.google.gerrit.client.ui.HintTextBox; +import com.google.gerrit.client.ui.RPCSuggestOracle; +import com.google.gwt.event.dom.client.ClickEvent; +import com.google.gwt.event.dom.client.ClickHandler; +import com.google.gwt.event.dom.client.KeyCodes; +import com.google.gwt.event.dom.client.KeyPressEvent; +import com.google.gwt.event.dom.client.KeyPressHandler; +import com.google.gwt.event.logical.shared.SelectionEvent; +import com.google.gwt.event.logical.shared.SelectionHandler; +import com.google.gwt.user.client.ui.Button; +import com.google.gwt.user.client.ui.Composite; +import com.google.gwt.user.client.ui.FlowPanel; +import com.google.gwt.user.client.ui.SuggestBox; +import com.google.gwt.user.client.ui.SuggestOracle.Suggestion; + +public class AddIncludedGroupBox extends Composite { + private final FlowPanel addPanel; + private final Button addMember; + private final HintTextBox nameTxtBox; + private final SuggestBox nameTxt; + private boolean submitOnSelection; + + public AddIncludedGroupBox() { + addPanel = new FlowPanel(); + addMember = new Button(Util.C.buttonAddIncludedGroup()); + nameTxtBox = new HintTextBox(); + nameTxt = new SuggestBox(new RPCSuggestOracle( + new AccountGroupSuggestOracle()), nameTxtBox); + + nameTxtBox.setVisibleLength(50); + nameTxtBox.setHintText(Util.C.defaultAccountGroupName()); + nameTxtBox.addKeyPressHandler(new KeyPressHandler() { + @Override + public void onKeyPress(KeyPressEvent event) { + submitOnSelection = false; + + if (event.getCharCode() == KeyCodes.KEY_ENTER) { + if (nameTxt.isSuggestionListShowing()) { + submitOnSelection = true; + } else { + doAdd(); + } + } + } + }); + nameTxt.addSelectionHandler(new SelectionHandler<Suggestion>() { + @Override + public void onSelection(SelectionEvent<Suggestion> event) { + if (submitOnSelection) { + submitOnSelection = false; + doAdd(); + } + } + }); + + addPanel.add(nameTxt); + addPanel.add(addMember); + + initWidget(addPanel); + } + + public void setAddButtonText(final String text) { + addMember.setText(text); + } + + public void addClickHandler(final ClickHandler handler) { + addMember.addClickHandler(handler); + } + + public String getText() { + String s = nameTxtBox.getText(); + return s == null ? "" : s; + } + + public void setEnabled(boolean enabled) { + addMember.setEnabled(enabled); + nameTxtBox.setEnabled(enabled); + } + + public void setText(String text) { + nameTxtBox.setText(text); + } + + private void doAdd() { + addMember.fireEvent(new ClickEvent() {}); + } +} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/BaseServiceImplementation.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/BaseServiceImplementation.java index 6bb905f..8f9d96e 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/BaseServiceImplementation.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/BaseServiceImplementation.java
@@ -87,8 +87,7 @@ } } catch (Failure e) { if (e.getCause() instanceof NoSuchProjectException - || e.getCause() instanceof NoSuchChangeException - || e.getCause() instanceof NoSuchGroupException) { + || e.getCause() instanceof NoSuchChangeException) { callback.onFailure(new NoSuchEntityException()); } else { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java index e4db047..d5cd0ff 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/CreateGroup.java
@@ -25,6 +25,8 @@ import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; +import java.util.Collections; + class CreateGroup extends Handler<AccountGroup.Id> { interface Factory { CreateGroup create(String groupName); @@ -46,6 +48,6 @@ public AccountGroup.Id call() throws OrmException, NameAlreadyUsedException { final PerformCreateGroup performCreateGroup = performCreateGroupFactory.create(); final Account.Id me = user.getAccountId(); - return performCreateGroup.createGroup(groupName, null, false, null, me); + return performCreateGroup.createGroup(groupName, null, false, null, Collections.singleton(me), null); } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java index 0b59ea2..b3918d3 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java
@@ -25,6 +25,8 @@ import com.google.gerrit.httpd.rpc.BaseServiceImplementation; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; +import com.google.gerrit.reviewdb.AccountGroupIncludeAudit; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gerrit.reviewdb.AccountGroupMemberAudit; import com.google.gerrit.reviewdb.ReviewDb; @@ -33,6 +35,7 @@ import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupControl; +import com.google.gerrit.server.account.GroupIncludeCache; import com.google.gerrit.server.account.Realm; import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwtjsonrpc.client.VoidResult; @@ -54,6 +57,7 @@ private final AccountResolver accountResolver; private final Realm accountRealm; private final GroupCache groupCache; + private final GroupIncludeCache groupIncludeCache; private final GroupControl.Factory groupControlFactory; private final CreateGroup.Factory createGroupFactory; @@ -63,8 +67,10 @@ @Inject GroupAdminServiceImpl(final Provider<ReviewDb> schema, final Provider<IdentifiedUser> currentUser, - final AccountCache accountCache, final AccountResolver accountResolver, - final Realm accountRealm, final GroupCache groupCache, + final AccountCache accountCache, + final GroupIncludeCache groupIncludeCache, + final AccountResolver accountResolver, final Realm accountRealm, + final GroupCache groupCache, final GroupControl.Factory groupControlFactory, final CreateGroup.Factory createGroupFactory, final RenameGroup.Factory renameGroupFactory, @@ -72,6 +78,7 @@ super(schema, currentUser); this.identifiedUser = currentUser; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; this.accountResolver = accountResolver; this.accountRealm = accountRealm; this.groupCache = groupCache; @@ -203,8 +210,8 @@ public void searchExternalGroups(final String searchFilter, final AsyncCallback<List<AccountGroup.ExternalNameKey>> callback) { final ArrayList<AccountGroup.ExternalNameKey> matches = - new ArrayList<AccountGroup.ExternalNameKey>(accountRealm - .lookupGroups(searchFilter)); + new ArrayList<AccountGroup.ExternalNameKey>( + accountRealm.lookupGroups(searchFilter)); Collections.sort(matches, new Comparator<AccountGroup.ExternalNameKey>() { @Override public int compare(AccountGroup.ExternalNameKey a, @@ -229,7 +236,7 @@ if (!a.isActive()) { throw new Failure(new InactiveAccountException(a.getFullName())); } - if (!control.canAdd(a.getId())) { + if (!control.canAddMember(a.getId())) { throw new Failure(new NoSuchEntityException()); } @@ -250,6 +257,38 @@ }); } + public void addGroupInclude(final AccountGroup.Id groupId, + final String groupName, final AsyncCallback<GroupDetail> callback) { + run(callback, new Action<GroupDetail>() { + public GroupDetail run(ReviewDb db) throws OrmException, Failure, + NoSuchGroupException { + final GroupControl control = groupControlFactory.validateFor(groupId); + if (control.getAccountGroup().getType() != AccountGroup.Type.INTERNAL) { + throw new Failure(new NameAlreadyUsedException()); + } + + final AccountGroup a = findGroup(groupName); + if (!control.canAddGroup(a.getId())) { + throw new Failure(new NoSuchEntityException()); + } + + final AccountGroupInclude.Key key = + new AccountGroupInclude.Key(groupId, a.getId()); + AccountGroupInclude m = db.accountGroupIncludes().get(key); + if (m == null) { + m = new AccountGroupInclude(key); + db.accountGroupIncludesAudit().insert( + Collections.singleton(new AccountGroupIncludeAudit(m, + getAccountId()))); + db.accountGroupIncludes().insert(Collections.singleton(m)); + groupIncludeCache.evictInclude(a.getId()); + } + + return groupDetailFactory.create(groupId).call(); + } + }); + } + public void deleteGroupMembers(final AccountGroup.Id groupId, final Set<AccountGroupMember.Key> keys, final AsyncCallback<VoidResult> callback) { @@ -271,7 +310,7 @@ for (final AccountGroupMember.Key k : keys) { final AccountGroupMember m = db.accountGroupMembers().get(k); if (m != null) { - if (!control.canRemove(m.getAccountId())) { + if (!control.canRemoveMember(m.getAccountId())) { throw new Failure(new NoSuchEntityException()); } @@ -304,6 +343,56 @@ }); } + public void deleteGroupIncludes(final AccountGroup.Id groupId, + final Set<AccountGroupInclude.Key> keys, + final AsyncCallback<VoidResult> callback) { + run(callback, new Action<VoidResult>() { + public VoidResult run(final ReviewDb db) throws OrmException, + NoSuchGroupException, Failure { + final GroupControl control = groupControlFactory.validateFor(groupId); + if (control.getAccountGroup().getType() != AccountGroup.Type.INTERNAL) { + throw new Failure(new NameAlreadyUsedException()); + } + + for (final AccountGroupInclude.Key k : keys) { + if (!groupId.equals(k.getGroupId())) { + throw new Failure(new NoSuchEntityException()); + } + } + + final Account.Id me = getAccountId(); + for (final AccountGroupInclude.Key k : keys) { + final AccountGroupInclude m = + db.accountGroupIncludes().get(k); + if (m != null) { + if (!control.canRemoveGroup(m.getIncludeId())) { + throw new Failure(new NoSuchEntityException()); + } + + AccountGroupIncludeAudit audit = null; + for (AccountGroupIncludeAudit a : db + .accountGroupIncludesAudit().byGroupInclude( + m.getGroupId(), m.getIncludeId())) { + if (a.isActive()) { + audit = a; + break; + } + } + + if (audit != null) { + audit.removed(me); + db.accountGroupIncludesAudit().update( + Collections.singleton(audit)); + } + db.accountGroupIncludes().delete(Collections.singleton(m)); + groupIncludeCache.evictInclude(m.getIncludeId()); + } + } + return VoidResult.INSTANCE; + } + }); + } + private void assertAmGroupOwner(final ReviewDb db, final AccountGroup group) throws Failure { try { @@ -323,4 +412,14 @@ } return r; } + + private AccountGroup findGroup(final String name) throws OrmException, + Failure { + final AccountGroup g = groupCache.get(new AccountGroup.NameKey(name)); + if (g == null) { + throw new Failure(new NoSuchGroupException(name)); + } + return g; + } + } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java index 28352a8..da6ab65 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupDetailFactory.java
@@ -19,11 +19,13 @@ import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.account.AccountInfoCacheFactory; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupControl; +import com.google.gerrit.server.account.GroupInfoCacheFactory; import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; @@ -42,6 +44,7 @@ private final GroupControl.Factory groupControl; private final GroupCache groupCache; private final AccountInfoCacheFactory aic; + private final GroupInfoCacheFactory gic; private final AccountGroup.Id groupId; private GroupControl control; @@ -50,11 +53,13 @@ GroupDetailFactory(final ReviewDb db, final GroupControl.Factory groupControl, final GroupCache groupCache, final AccountInfoCacheFactory.Factory accountInfoCacheFactory, + final GroupInfoCacheFactory.Factory groupInfoCacheFactory, @Assisted final AccountGroup.Id groupId) { this.db = db; this.groupControl = groupControl; this.groupCache = groupCache; this.aic = accountInfoCacheFactory.create(); + this.gic = groupInfoCacheFactory.create(); this.groupId = groupId; } @@ -69,17 +74,19 @@ switch (group.getType()) { case INTERNAL: detail.setMembers(loadMembers()); + detail.setIncludes(loadIncludes()); break; } detail.setAccounts(aic.create()); detail.setCanModify(control.isOwner()); + detail.setGroups(gic.create()); return detail; } private List<AccountGroupMember> loadMembers() throws OrmException { List<AccountGroupMember> members = new ArrayList<AccountGroupMember>(); for (final AccountGroupMember m : db.accountGroupMembers().byGroup(groupId)) { - if (control.canSee(m.getAccountId())) { + if (control.canSeeMember(m.getAccountId())) { aic.want(m.getAccountId()); members.add(m); } @@ -109,4 +116,40 @@ }); return members; } + + private List<AccountGroupInclude> loadIncludes() throws OrmException { + List<AccountGroupInclude> groups = new ArrayList<AccountGroupInclude>(); + + for (final AccountGroupInclude m : db.accountGroupIncludes().byGroup(groupId)) { + if (control.canSeeGroup(m.getIncludeId())) { + gic.want(m.getIncludeId()); + groups.add(m); + } + } + + Collections.sort(groups, new Comparator<AccountGroupInclude>() { + public int compare(final AccountGroupInclude o1, + final AccountGroupInclude o2) { + final AccountGroup a = gic.get(o1.getIncludeId()); + final AccountGroup b = gic.get(o2.getIncludeId()); + return n(a).compareTo(n(b)); + } + + private String n(final AccountGroup a) { + String n = a.getName(); + if (n != null && n.length() > 0) { + return n; + } + + n = a.getDescription(); + if (n != null && n.length() > 0) { + return n; + } + + return a.getId().toString(); + } + }); + + return groups; + } } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupInclude.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupInclude.java new file mode 100644 index 0000000..86b0966 --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupInclude.java
@@ -0,0 +1,81 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.reviewdb; + +import com.google.gwtorm.client.Column; +import com.google.gwtorm.client.CompoundKey; + +/** Membership of an {@link AccountGroup} in an {@link AccountGroup}. */ +public final class AccountGroupInclude { + public static class Key extends CompoundKey<AccountGroup.Id> { + private static final long serialVersionUID = 1L; + + @Column(id = 1) + protected AccountGroup.Id groupId; + + @Column(id = 2) + protected AccountGroup.Id includeId; + + protected Key() { + groupId = new AccountGroup.Id(); + includeId = new AccountGroup.Id(); + } + + public Key(final AccountGroup.Id g, final AccountGroup.Id i) { + groupId = g; + includeId = i; + } + + @Override + public AccountGroup.Id getParentKey() { + return groupId; + } + + public AccountGroup.Id getGroupId() { + return groupId; + } + + public AccountGroup.Id getIncludeId() { + return includeId; + } + + @Override + public com.google.gwtorm.client.Key<?>[] members() { + return new com.google.gwtorm.client.Key<?>[] {includeId}; + } + } + + @Column(id = 1, name = Column.NONE) + protected Key key; + + protected AccountGroupInclude() { + } + + public AccountGroupInclude(final AccountGroupInclude.Key k) { + key = k; + } + + public AccountGroupInclude.Key getKey() { + return key; + } + + public AccountGroup.Id getGroupId() { + return key.groupId; + } + + public AccountGroup.Id getIncludeId() { + return key.includeId; + } +} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAccess.java new file mode 100644 index 0000000..4881635 --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAccess.java
@@ -0,0 +1,33 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.reviewdb; + +import com.google.gwtorm.client.Access; +import com.google.gwtorm.client.OrmException; +import com.google.gwtorm.client.PrimaryKey; +import com.google.gwtorm.client.Query; +import com.google.gwtorm.client.ResultSet; + +public interface AccountGroupIncludeAccess extends + Access<AccountGroupInclude, AccountGroupInclude.Key> { + @PrimaryKey("key") + AccountGroupInclude get(AccountGroupInclude.Key key) throws OrmException; + + @Query("WHERE key.includeId = ?") + ResultSet<AccountGroupInclude> byInclude(AccountGroup.Id id) throws OrmException; + + @Query("WHERE key.groupId = ?") + ResultSet<AccountGroupInclude> byGroup(AccountGroup.Id id) throws OrmException; +} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAudit.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAudit.java new file mode 100644 index 0000000..e99e480 --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAudit.java
@@ -0,0 +1,97 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.reviewdb; + +import com.google.gwtorm.client.Column; +import com.google.gwtorm.client.CompoundKey; + +import java.sql.Timestamp; + +/** Inclusion of an {@link AccountGroup} in another {@link AccountGroup}. */ +public final class AccountGroupIncludeAudit { + public static class Key extends CompoundKey<AccountGroup.Id> { + private static final long serialVersionUID = 1L; + + @Column(id = 1) + protected AccountGroup.Id groupId; + + @Column(id = 2) + protected AccountGroup.Id includeId; + + @Column(id = 3) + protected Timestamp addedOn; + + protected Key() { + groupId = new AccountGroup.Id(); + includeId = new AccountGroup.Id(); + } + + public Key(final AccountGroup.Id g, final AccountGroup.Id i, final Timestamp t) { + groupId = g; + includeId = i; + addedOn = t; + } + + @Override + public AccountGroup.Id getParentKey() { + return groupId; + } + + @Override + public com.google.gwtorm.client.Key<?>[] members() { + return new com.google.gwtorm.client.Key<?>[] {includeId}; + } + } + + @Column(id = 1, name = Column.NONE) + protected Key key; + + @Column(id = 2) + protected Account.Id addedBy; + + @Column(id = 3, notNull = false) + protected Account.Id removedBy; + + @Column(id = 4, notNull = false) + protected Timestamp removedOn; + + protected AccountGroupIncludeAudit() { + } + + public AccountGroupIncludeAudit(final AccountGroupInclude m, + final Account.Id adder) { + final AccountGroup.Id group = m.getGroupId(); + final AccountGroup.Id include = m.getIncludeId(); + key = new AccountGroupIncludeAudit.Key(group, include, now()); + addedBy = adder; + } + + public AccountGroupIncludeAudit.Key getKey() { + return key; + } + + public boolean isActive() { + return removedOn == null; + } + + public void removed(final Account.Id deleter) { + removedBy = deleter; + removedOn = now(); + } + + private static Timestamp now() { + return new Timestamp(System.currentTimeMillis()); + } +} \ No newline at end of file diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAuditAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAuditAccess.java new file mode 100644 index 0000000..55b50e8 --- /dev/null +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountGroupIncludeAuditAccess.java
@@ -0,0 +1,32 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.reviewdb; + +import com.google.gwtorm.client.Access; +import com.google.gwtorm.client.OrmException; +import com.google.gwtorm.client.PrimaryKey; +import com.google.gwtorm.client.Query; +import com.google.gwtorm.client.ResultSet; + +public interface AccountGroupIncludeAuditAccess extends + Access<AccountGroupIncludeAudit, AccountGroupIncludeAudit.Key> { + @PrimaryKey("key") + AccountGroupIncludeAudit get(AccountGroupIncludeAudit.Key key) + throws OrmException; + + @Query("WHERE key.groupId = ? AND key.includeId = ?") + ResultSet<AccountGroupIncludeAudit> byGroupInclude(AccountGroup.Id groupId, + AccountGroup.Id incGroupId) throws OrmException; +} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java index f9b3cfa..49e07ff 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/ReviewDb.java
@@ -73,6 +73,12 @@ AccountGroupMemberAuditAccess accountGroupMembersAudit(); @Relation + AccountGroupIncludeAccess accountGroupIncludes(); + + @Relation + AccountGroupIncludeAuditAccess accountGroupIncludesAudit(); + + @Relation AccountGroupAgreementAccess accountGroupAgreements(); @Relation
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql index 0d41729..d33d24d 100644 --- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql +++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_generic.sql
@@ -45,6 +45,13 @@ -- ********************************************************************* +-- AccountGroupIncludeAccess +-- @PrimaryKey covers: byGroup +CREATE INDEX account_group_includes_byInclude +ON account_group_includes (include_id); + + +-- ********************************************************************* -- AccountProjectWatchAccess -- @PrimaryKey covers: byAccount -- covers: byProject
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql index b44351c..cb8196e 100644 --- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql +++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/index_postgres.sql
@@ -84,6 +84,13 @@ -- ********************************************************************* +-- AccountGroupIncludeAccess +-- @PrimaryKey covers: byGroup +CREATE INDEX account_group_includes_byInclude +ON account_group_includes (include_id); + + +-- ********************************************************************* -- AccountProjectWatchAccess -- @PrimaryKey covers: byAccount -- covers: byProject
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java index 78cbed3..e46957d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
@@ -23,6 +23,7 @@ import com.google.gerrit.reviewdb.StarredChange; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; +import com.google.gerrit.server.account.GroupIncludeCache; import com.google.gerrit.server.account.Realm; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.CanonicalWebUrl; @@ -46,7 +47,9 @@ import java.util.Collections; import java.util.Date; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; +import java.util.Queue; import java.util.Set; import java.util.TimeZone; @@ -61,15 +64,18 @@ private final Provider<String> canonicalUrl; private final Realm realm; private final AccountCache accountCache; + private final GroupIncludeCache groupIncludeCache; @Inject GenericFactory(final AuthConfig authConfig, final @CanonicalWebUrl Provider<String> canonicalUrl, - final Realm realm, final AccountCache accountCache) { + final Realm realm, final AccountCache accountCache, + final GroupIncludeCache groupIncludeCache) { this.authConfig = authConfig; this.canonicalUrl = canonicalUrl; this.realm = realm; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; } public IdentifiedUser create(final Account.Id id) { @@ -78,13 +84,13 @@ public IdentifiedUser create(Provider<ReviewDb> db, Account.Id id) { return new IdentifiedUser(AccessPath.UNKNOWN, authConfig, canonicalUrl, - realm, accountCache, null, db, id); + realm, accountCache, groupIncludeCache, null, db, id); } public IdentifiedUser create(AccessPath accessPath, Provider<SocketAddress> remotePeerProvider, Account.Id id) { return new IdentifiedUser(accessPath, authConfig, canonicalUrl, realm, - accountCache, remotePeerProvider, null, id); + accountCache, groupIncludeCache, remotePeerProvider, null, id); } } @@ -100,6 +106,7 @@ private final Provider<String> canonicalUrl; private final Realm realm; private final AccountCache accountCache; + private final GroupIncludeCache groupIncludeCache; private final Provider<SocketAddress> remotePeerProvider; private final Provider<ReviewDb> dbProvider; @@ -108,6 +115,7 @@ RequestFactory(final AuthConfig authConfig, final @CanonicalWebUrl Provider<String> canonicalUrl, final Realm realm, final AccountCache accountCache, + final GroupIncludeCache groupIncludeCache, final @RemotePeer Provider<SocketAddress> remotePeerProvider, final Provider<ReviewDb> dbProvider) { @@ -115,6 +123,7 @@ this.canonicalUrl = canonicalUrl; this.realm = realm; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; this.remotePeerProvider = remotePeerProvider; this.dbProvider = dbProvider; @@ -123,7 +132,7 @@ public IdentifiedUser create(final AccessPath accessPath, final Account.Id id) { return new IdentifiedUser(accessPath, authConfig, canonicalUrl, realm, - accountCache, remotePeerProvider, dbProvider, id); + accountCache, groupIncludeCache, remotePeerProvider, dbProvider, id); } } @@ -133,6 +142,7 @@ private final Provider<String> canonicalUrl; private final Realm realm; private final AccountCache accountCache; + private final GroupIncludeCache groupIncludeCache; @Nullable private final Provider<SocketAddress> remotePeerProvider; @@ -151,12 +161,14 @@ private IdentifiedUser(final AccessPath accessPath, final AuthConfig authConfig, final Provider<String> canonicalUrl, final Realm realm, final AccountCache accountCache, + final GroupIncludeCache groupIncludeCache, @Nullable final Provider<SocketAddress> remotePeerProvider, @Nullable final Provider<ReviewDb> dbProvider, final Account.Id id) { super(accessPath, authConfig); this.canonicalUrl = canonicalUrl; this.realm = realm; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; this.remotePeerProvider = remotePeerProvider; this.dbProvider = dbProvider; this.accountId = id; @@ -207,14 +219,35 @@ @Override public Set<AccountGroup.Id> getEffectiveGroups() { if (effectiveGroups == null) { - if (authConfig.isIdentityTrustable(state().getExternalIds())) { - effectiveGroups = realm.groups(state()); + Set<AccountGroup.Id> seedGroups; + if (authConfig.isIdentityTrustable(state().getExternalIds())) { + seedGroups = realm.groups(state()); } else { - effectiveGroups = authConfig.getRegisteredGroups(); + seedGroups = authConfig.getRegisteredGroups(); + } + + effectiveGroups = getIncludedGroups(seedGroups); + } + + return effectiveGroups; + } + + private Set<AccountGroup.Id> getIncludedGroups(Set<AccountGroup.Id> seedGroups) { + Set<AccountGroup.Id> includes = new HashSet<AccountGroup.Id> (seedGroups); + Queue<AccountGroup.Id> groupQueue = new LinkedList<AccountGroup.Id> (seedGroups); + + while (groupQueue.size() > 0) { + AccountGroup.Id id = groupQueue.remove(); + + for (final AccountGroup.Id groupId : groupIncludeCache.getByInclude(id)) { + if (includes.add(groupId)) { + groupQueue.add(groupId); + } } } - return effectiveGroups; + + return Collections.unmodifiableSet(includes); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java index 20f9ec2..602b593 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
@@ -83,15 +83,27 @@ || getCurrentUser().isAdministrator(); } - public boolean canAdd(final Account.Id id) { + public boolean canAddMember(final Account.Id id) { return isOwner(); } - public boolean canRemove(final Account.Id id) { + public boolean canRemoveMember(final Account.Id id) { return isOwner(); } - public boolean canSee(Account.Id id) { + public boolean canSeeMember(Account.Id id) { + return isVisible(); + } + + public boolean canAddGroup(final AccountGroup.Id id) { + return isOwner(); + } + + public boolean canRemoveGroup(final AccountGroup.Id id) { + return isOwner(); + } + + public boolean canSeeGroup(AccountGroup.Id id) { return isVisible(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java new file mode 100644 index 0000000..3088806 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCache.java
@@ -0,0 +1,27 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.account; + +import com.google.gerrit.reviewdb.AccountGroup; + +import java.util.Collection; + +/** Tracks group inclusions in memory for efficient access. */ +public interface GroupIncludeCache { + public Collection<AccountGroup.Id> getByInclude(AccountGroup.Id groupId); + + public void evictInclude(AccountGroup.Id groupId); +} + diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java new file mode 100644 index 0000000..76e9231 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java
@@ -0,0 +1,98 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.account; + +import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; +import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gerrit.server.cache.Cache; +import com.google.gerrit.server.cache.CacheModule; +import com.google.gerrit.server.cache.EntryCreator; +import com.google.gwtorm.client.SchemaFactory; + +import com.google.inject.Inject; +import com.google.inject.Module; +import com.google.inject.Singleton; +import com.google.inject.TypeLiteral; +import com.google.inject.name.Named; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; + +/** Tracks group inclusions in memory for efficient access. */ +@Singleton +public class GroupIncludeCacheImpl implements GroupIncludeCache { + private static final String BYINCLUDE_NAME = "groups_byinclude"; + + public static Module module() { + return new CacheModule() { + @Override + protected void configure() { + final TypeLiteral<Cache<AccountGroup.Id, Collection<AccountGroup.Id>>> byInclude = + new TypeLiteral<Cache<AccountGroup.Id, Collection<AccountGroup.Id>>>() {}; + core(byInclude, BYINCLUDE_NAME).populateWith(ByIncludeLoader.class); + + bind(GroupIncludeCacheImpl.class); + bind(GroupIncludeCache.class).to(GroupIncludeCacheImpl.class); + } + }; + } + + private final Cache<AccountGroup.Id, Collection<AccountGroup.Id>> byInclude; + + @Inject + GroupIncludeCacheImpl( + @Named(BYINCLUDE_NAME) Cache<AccountGroup.Id, Collection<AccountGroup.Id>> byInclude) { + this.byInclude = byInclude; + } + + public Collection<AccountGroup.Id> getByInclude(final AccountGroup.Id groupId) { + return byInclude.get(groupId); + } + + public void evictInclude(AccountGroup.Id groupId) { + byInclude.remove(groupId); + } + + static class ByIncludeLoader extends EntryCreator<AccountGroup.Id, Collection<AccountGroup.Id>> { + private final SchemaFactory<ReviewDb> schema; + + @Inject + ByIncludeLoader(final SchemaFactory<ReviewDb> sf) { + schema = sf; + } + + @Override + public Collection<AccountGroup.Id> createEntry(final AccountGroup.Id key) throws Exception { + final ReviewDb db = schema.open(); + try { + ArrayList<AccountGroup.Id> groupArray = new ArrayList<AccountGroup.Id> (); + for (AccountGroupInclude agi : db.accountGroupIncludes().byInclude(key)) { + groupArray.add(agi.getGroupId()); + } + + return Collections.unmodifiableCollection(groupArray); + } finally { + db.close(); + } + } + + @Override + public Collection<AccountGroup.Id> missing(final AccountGroup.Id key) { + return Collections.emptyList(); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java new file mode 100644 index 0000000..a3e592f --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupInfoCacheFactory.java
@@ -0,0 +1,75 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.account; + +import com.google.gerrit.common.data.GroupInfo; +import com.google.gerrit.common.data.GroupInfoCache; +import com.google.gerrit.reviewdb.AccountGroup; +import com.google.inject.Inject; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** Efficiently builds a {@link GroupInfoCache}. */ +public class GroupInfoCacheFactory { + public interface Factory { + GroupInfoCacheFactory create(); + } + + private final GroupCache groupCache; + private final Map<AccountGroup.Id, AccountGroup> out; + + @Inject + GroupInfoCacheFactory(final GroupCache groupCache) { + this.groupCache = groupCache; + this.out = new HashMap<AccountGroup.Id, AccountGroup>(); + } + + /** + * Indicate a group will be needed later on. + * + * @param id identity that will be needed in the future; may be null. + */ + public void want(final AccountGroup.Id id) { + if (id != null && !out.containsKey(id)) { + out.put(id, groupCache.get(id)); + } + } + + /** Indicate one or more groups will be needed later on. */ + public void want(final Iterable<AccountGroup.Id> ids) { + for (final AccountGroup.Id id : ids) { + want(id); + } + } + + public AccountGroup get(final AccountGroup.Id id) { + want(id); + return out.get(id); + } + + /** + * Create an GroupInfoCache with the currently loaded AccountGroup entities. + * */ + public GroupInfoCache create() { + final List<GroupInfo> r = new ArrayList<GroupInfo>(out.size()); + for (final AccountGroup a : out.values()) { + r.add(new GroupInfo(a)); + } + return new GroupInfoCache(r); + } +} \ No newline at end of file diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java index 8e5d274..aab1cda 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PerformCreateGroup.java
@@ -17,6 +17,8 @@ import com.google.gerrit.common.errors.NameAlreadyUsedException; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountGroup; +import com.google.gerrit.reviewdb.AccountGroupInclude; +import com.google.gerrit.reviewdb.AccountGroupIncludeAudit; import com.google.gerrit.reviewdb.AccountGroupMember; import com.google.gerrit.reviewdb.AccountGroupMemberAudit; import com.google.gerrit.reviewdb.AccountGroupName; @@ -27,6 +29,7 @@ import com.google.inject.Inject; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; @@ -38,13 +41,15 @@ private final ReviewDb db; private final AccountCache accountCache; + private final GroupIncludeCache groupIncludeCache; private final IdentifiedUser currentUser; @Inject PerformCreateGroup(final ReviewDb db, final AccountCache accountCache, - final IdentifiedUser currentUser) { + final GroupIncludeCache groupIncludeCache, final IdentifiedUser currentUser) { this.db = db; this.accountCache = accountCache; + this.groupIncludeCache = groupIncludeCache; this.currentUser = currentUser; } @@ -60,6 +65,7 @@ * @param ownerGroupId the group that should own the new group, if * <code>null</code> the new group will own itself * @param initialMembers initial members to be added to the new group + * @param initialGroups initial groups to include in the new group * @return the id of the new group * @throws OrmException is thrown in case of any data store read or write * error @@ -68,7 +74,9 @@ */ public AccountGroup.Id createGroup(final String groupName, final String groupDescription, final boolean visibleToAll, - final AccountGroup.Id ownerGroupId, final Account.Id... initialMembers) + final AccountGroup.Id ownerGroupId, + final Collection<? extends Account.Id> initialMembers, + final Collection<? extends AccountGroup.Id> initialGroups) throws OrmException, NameAlreadyUsedException { final AccountGroup.Id groupId = new AccountGroup.Id(db.nextAccountGroupId()); @@ -93,11 +101,15 @@ addMembers(groupId, initialMembers); + if (initialGroups != null) { + addGroups(groupId, initialGroups); + } + return groupId; } private void addMembers(final AccountGroup.Id groupId, - final Account.Id... members) throws OrmException { + final Collection<? extends Account.Id> members) throws OrmException { final List<AccountGroupMember> memberships = new ArrayList<AccountGroupMember>(); final List<AccountGroupMemberAudit> membershipsAudit = @@ -119,4 +131,26 @@ } } + private void addGroups(final AccountGroup.Id groupId, + final Collection<? extends AccountGroup.Id> groups) throws OrmException { + final List<AccountGroupInclude> includeList = + new ArrayList<AccountGroupInclude>(); + final List<AccountGroupIncludeAudit> includesAudit = + new ArrayList<AccountGroupIncludeAudit>(); + for (AccountGroup.Id includeId : groups) { + final AccountGroupInclude groupInclude = + new AccountGroupInclude(new AccountGroupInclude.Key(groupId, includeId)); + includeList.add(groupInclude); + + final AccountGroupIncludeAudit audit = + new AccountGroupIncludeAudit(groupInclude, currentUser.getAccountId()); + includesAudit.add(audit); + } + db.accountGroupIncludes().insert(includeList); + db.accountGroupIncludesAudit().insert(includesAudit); + + for (AccountGroup.Id includeId : groups) { + groupIncludeCache.evictInclude(includeId); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index f77550e..df10c27 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -35,6 +35,8 @@ import com.google.gerrit.server.account.DefaultRealm; import com.google.gerrit.server.account.EmailExpander; import com.google.gerrit.server.account.GroupCacheImpl; +import com.google.gerrit.server.account.GroupInfoCacheFactory; +import com.google.gerrit.server.account.GroupIncludeCacheImpl; import com.google.gerrit.server.account.Realm; import com.google.gerrit.server.auth.ldap.LdapModule; import com.google.gerrit.server.cache.CachePool; @@ -153,11 +155,13 @@ install(AccountByEmailCacheImpl.module()); install(AccountCacheImpl.module()); install(GroupCacheImpl.module()); + install(GroupIncludeCacheImpl.module()); install(PatchListCacheImpl.module()); install(ProjectCacheImpl.module()); install(new AccessControlModule()); factory(AccountInfoCacheFactory.Factory.class); + factory(GroupInfoCacheFactory.Factory.class); factory(ProjectState.Factory.class); factory(RefControl.Factory.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index fb5f1f8..ecd446d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -32,7 +32,7 @@ /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - private static final Class<? extends SchemaVersion> C = Schema_50.class; + private static final Class<? extends SchemaVersion> C = Schema_51.class; public static class Module extends AbstractModule { @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_51.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_51.java new file mode 100644 index 0000000..d111160 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_51.java
@@ -0,0 +1,41 @@ +// Copyright (C) 2011 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.schema; + +import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gwtorm.jdbc.JdbcSchema; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import java.sql.SQLException; +import java.sql.Statement; + +public class Schema_51 extends SchemaVersion { + @Inject + Schema_51(Provider<Schema_50> prior) { + super(prior); + } + + @Override + protected void migrateData(ReviewDb db, UpdateUI ui) throws SQLException { + Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); + try { + stmt.execute("CREATE INDEX account_group_includes_byInclude" + + " ON account_group_includes (include_id)"); + } finally { + stmt.close(); + } + } +} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java index b239fab..36e064b 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/AdminCreateGroup.java
@@ -58,6 +58,13 @@ @Option(name = "--visible-to-all", usage = "to make the group visible to all registered users") private boolean visibleToAll; + private final Set<AccountGroup.Id> initialGroups = new HashSet<AccountGroup.Id>(); + + @Option(name = "--group", aliases = "-g", metaVar = "GROUP", usage = "initial set of groups to be included in the group") + void addGroup(final AccountGroup.Id id) { + initialGroups.add(id); + } + @Inject private PerformCreateGroup.Factory performCreateGroupFactory; @@ -77,8 +84,7 @@ performCreateGroupFactory.create(); try { performCreateGroup.createGroup(groupName, groupDescription, visibleToAll, - ownerGroupId, - initialMembers.toArray(new Account.Id[initialMembers.size()])); + ownerGroupId, initialMembers, initialGroups); } catch (NameAlreadyUsedException e) { throw die(e); } 